Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Apr 11, 2019

Summary

Possible corrections are as follows:

  • In remarks section of documentation for Marshal.GetEndComSlot, the following possible correction is made:

... If the class interface is auto-dispatch, this method always returns -1 to indicate that the dispatch-only interface does not expose a v-table to managed clients. ..._

  • In parameter descriptions of documentation for Marshal.GetStartComSlot, the following possible correction is made:
A type that represents an interface or class.

Justifications for suggestions

  • Potential corrections align remarks section & parameter descriptions of the two methods: Marshal.GetEndComSlot & Marshal.GetStartComSlot.
  • I've written code where the parameter in question has been a class type for the Marshal.GetStartComSlot method, and it worked without problems.
  • The remarks section for Marshal.GetStartComSlot says that it returns a v-table number for an interface or a class. It logically follows that the one parameter that the static method accepts can be either an interface or a class.
  • Changing 'auto-dual' to 'auto-dispatch' seems to be correct if you read documentation here.
  • I've written code that runs Marshal.GetEndComSlot on an System.Collections.ArrayList type as well as on a System.Object type. The documentation for System.Object indicates that it is an auto-dual class ("[System.Runtime.InteropServices.ClassInterface(System.Runtime.InteropServices.ClassInterfaceType.AutoDual)]" is written in class definition). Running GetEndComSlot on this type does not return -1 which is what the previous documentation for the method indicated would be the case. In contrast, running the method on an ArrayList type returns -1. The documentation here indicates that if the ClassInterface attribute is not defined, its value should be the default 'auto-dispatch'. The ArrayList class definition in the documentation doesn't define the attribute and so should be 'auto-dispatch' (from my thinking). Therefore, according to the previous documentation for the GetEndComSlot, running the method on an ArrayList type probably should return a genuine positive-integer com slot, but as just said, it returns -1 (the number that is supposed to be returned by auto-dual classes according to previous documentation.) It seems the documentation for GetEndComSlot has got auto-dual mixed up with auto-dispatch.
  • The previous documentation for GetEndComSlot has the text: "If the class interface is auto-dual, this method always returns -1 to indicate that the dispatch-only interface does not expose a v-table to managed clients." I've suggested this text should be changed. The first part of the sentence refers to auto-dual classes, but the second part, which it seems is supposed to either also refer to auto-dual classes or an aspect of them, suddenly comments on dispatch-only interfaces which is not a proper reference to auto-dual classes or a direct aspect of them. Instead dispatch-only interfaces are connected to auto-dispatch classes which are not auto-dual classes - it looks like the first part of the sentence likely was supposed to refer to auto-dispatch classes instead of auto-dual classes.

Possible corrections are as follows:
 - In remarks section of documentation for Marshal.GetEndComSlot, the following possible correction is made: "... If the class interface is auto-dispatch, this method always returns -1 to indicate that the dispatch-only interface does not expose a v-table to managed clients. ..."
- In parameter descriptions of documentation for Marshal.GetStartComSlot, the following possible correction is made: "<param name="t">A type that represents an interface or class.</param>"

Justifications for suggestions:
- Potential corrections align remarks section & parameter descriptions of the two methods: Marshal.GetEndComSlot & Marshal.GetStartComSlot.
- I've written code where the parameter in question has been a class type for the Marshal.GetStartComSlot method, and it worked without problems.
- The remarks section for Marshal.GetStartComSlot says that it returns a v-table number for an interface or a class. It logically follows that the one parameter that the static method accepts can be either an interface or a class. 
- Changing 'auto-dual' to 'auto-dispatch' seems to be correct when reading documentation at: https://docs.microsoft.com/en-us/dotnet/framework/interop/com-callable-wrapper#avoid-caching-dispatch-identifiers-dispids.
- I've written code that runs Marshal.GetEndComSlot on an System.Collections.ArrayList type as well as on a System.Object type. The documentation for System.Object indicates that it is an auto-dual class ("[System.Runtime.InteropServices.ClassInterface(System.Runtime.InteropServices.ClassInterfaceType.AutoDual)]" is written in class definition). Running GetEndComSlot on this type does not return -1 which is what the previous documentation for the method indicated would be the case. Contrastedly, running the method on an ArrayList type returns -1.  The documentation at 'https://docs.microsoft.com/en-us/previous-versions/dotnet/netframework-4.0/4fcadw4a(v=vs.100)#avoid-caching-dispatch-identifiers-dispids' indicates that if the ClassInterface attribute is not defined, its value should be the default auto-dispatch. The ArrayList class definition in the documentation doesn't define the attribute and so should be auto-dispatch (from my thinking). Therefore, according to the previous documentation for the GetEndComSlot, running the method on an ArrayList type probably should return a genuine positive-integer com slot, but as just said, it returns -1 (the number that is supposed to be returned by auto-dual classes according to previous documentation.) It seems the documentation for GetEndComSlot has got auto-dual mixed up with auto-dispatch.
- The previous documentation for GetEndComSlot has the text: 'If the class interface is auto-dual, this method always returns -1 to indicate that the dispatch-only interface does not expose a v-table to managed clients.' I've suggested this text should be changed. The first part of the sentence refers to auto-dual classes, but the second part, which it seems is supposed to either also refer to auto-dual classes or an aspect of them, suddenly comments on dispatch-only interfaces which is not a proper reference to auto-dual classes or a direct aspect of them. Instead dispatch-only interfaces are connected to auto-dispatch classes which are not auto-dual classes - it looks like the first part of the sentence likely was supposed to refer to auto-dispatch classes instead of auto-dual classes.
@mairaw
Copy link
Contributor

mairaw commented Apr 12, 2019

@AaronRobinsonMSFT, @jkoritzinsky can one of you review the changes done here?

@mairaw mairaw assigned ghost Apr 12, 2019
@mairaw mairaw added the ✨ 1st-time dotnet-api-docs contributor! Indicates PRs from new contributors to the dotnet-api-docs repository label Apr 12, 2019
@mairaw mairaw added this to the April 2019 milestone Apr 12, 2019
@mairaw
Copy link
Contributor

mairaw commented Jun 7, 2019

Friendly ping @AaronRobinsonMSFT, @jkoritzinsky

@mairaw mairaw added the waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews label Jun 7, 2019
@mairaw mairaw removed the waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews label Aug 6, 2019
Copy link
Contributor

@mairaw mairaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @markfern. Sorry for the delay on merging this PR. I've removed my review comment and I'll apply it in a separate PR.

@mairaw mairaw merged commit d8000bf into dotnet:master Aug 6, 2019
@mairaw mairaw mentioned this pull request Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ 1st-time dotnet-api-docs contributor! Indicates PRs from new contributors to the dotnet-api-docs repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants